🦋 Changeset detected

Latest commit: 54acbc0

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Looks great, but what about deserialization those objects?

Looks great, but what about deserialization those objects?

That's what the createSerialized$ does

Does this mean we might be able to serialize functions from other libraries, like vanilla js?

@Varixo @thejackshelton gaah I had forgotten to push the actual commit 😅

Does this mean we might be able to serialize functions from other libraries, like vanilla js?

Yes exactly. You need to add a symbol prop that serializes the value and then provide a deserializer that will get called lazily.

Open in Stackblitz

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@7223

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@7223

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@7223

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@7223

commit: 54acbc0

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

I tried the changes locally and I'm having the following issue when using props as initial data:

Plus a linting issue when using the SerializerSignal in a useTask$.

ESLint: 
When referencing "mapbox" inside a different scope (useTask$), Qwik needs to serialize the value, however "mapbox. force" is a function, which is not serializable. Check out https:// qwik. dev/ docs/ advanced/ dollar/ for more details.
(qwik/ valid-lexical-scope)

Link to MRE: https://github.com/ianlet/qwik-custom-serder-scope-issues/blob/main/src/components/qwik-mapbox.tsx

Edit: actually we won't be able to useSerializer for mapbox as it needs a window/document. Could have been nice though to render a map without having to wake up Qwik. In any case, it was an interesting exercise to try this new feature on a more complex object that we don't control (external lib).

Also, after playing with it a little bit, I think the deserialize function should not handle both the actual deserialization and the update when tracked values change. It's conceptually 2 different things and will require 2 different implementations anyway for developers who will use it. And the fact that we had to document 2 examples on how to use it (without tracking and with tracking) reinforces this idea.

So, IMO it should be 2 different callbacks as it is 2 different use cases, triggered after 2 different events.

I addressed all the comments I think.

@ianlet BTW even when only using mapbox on the client, this is still useful. Just make the deserialize isBrowser ? makeMapbox() : undefined and it will just work on the client when you need it.

@ianlet BTW even when only using mapbox on the client, this is still useful. Just make the deserialize isBrowser ? makeMapbox() : undefined and it will just work on the client when you need it.

I tried to use the new syntax with update and I changed it to apply your isServer suggestion but if I do so, the deserialize function is never called on the client so the value never gets initialized.

Also, having to handle the undefined everywhere in that case makes it a bit heavy, but it's a detail.

    const mapbox = useSerializer$<
      mapboxgl.Map | undefined,
      SerializationState | undefined
    >(() => ({
      initial: {
        token: token.value,
        mapStyle: mapStyle.value,
        center: center.value,
      },
      serialize: (map: mapboxgl.Map | undefined) => {
        if (!map) return;

        return {
          token,
          mapStyle: map.getStyle() || undefined,
          center: map.getCenter().toArray(),
        };
      },
      deserialize: (state: SerializationState | undefined) => {
        if (isServer) return;

        const s = state || { token: token.value, mapStyle: mapStyle.value, center: center.value };

        return new mapboxgl.Map({
          container: ref.value!,
          style: s.mapStyle,
          center: s.center,
        });
      },
      update: (map: mapboxgl.Map | undefined) => {
        if (!map) return;

        map.setStyle(mapStyle.value);
        map.setCenter(center.value);
      },
    }));

Note: ESLint is still not happy about tracking the SerializerSignal:

    useTask$(({ track }) => {
      const map = track(mapbox);

      map?.on("load", () => console.log("MAP LOADED"));
    });

5:28:38 PM [vite] warning: When referencing "mapbox" inside a different scope (useTask$), Qwik needs to serialize the value, however "mapbox.force" is a function, which is not serializable.
Check out https://qwik.dev/docs/advanced/dollar/ for more details.
  Plugin: vite-plugin-qwik
  File: qwik-custom-serder/src/components/qwik-mapbox.tsx:65:25
  63 |
  64 |      useTask$(({ track }) => {
  65 |        const map = track(mapbox);
     |                           ^
  66 |
  67 |        map?.on("load", () => console.log("MAP LOADED"));

I played more with it and if you change a value that is tracked more than once, the serialized value is only updated the first time: Playground #1

So I tried to change it to directly use the signal in the deserialize function (feels weird to me though), and this time it crashes if updated more than once: Playground #2

Uncaught (in promise) TypeError: p0.value is undefined
    qFuncs_za6h4ekne3o (index):1
    untrackedValue signal.js:405
    invokeApply use-core.js:62
    invoke use-core.js:54
    trackSignal use-core.js:130
    $computeIfNeeded$ signal.js:405
    returnValue scheduler.js:349
    retryOnPromise promises.js:88
    executeChore scheduler.js:348
    drainUpTo scheduler.js:212
    schedule scheduler.js:179
    queueQRL queue-qrl.js:21
    dispatch (index):140
    processDocumentEvent (index):162
    addEventListener (index):194
    processEventOrNode (index):202
    processEventOrNode (index):202
    <anonymous> (index):225

@ianlet Thank you for your playgrounds, the problem is that you didn't use foo.value in the deserializer, and so there's no subscription being created.

I added an eslint rule (yey vibe coding) that warns about this.